Skip to content

http,net,stream: reconcile read, write, and parser paths - #64980

Open
GetThatCookie wants to merge 3 commits into
nodejs:mainfrom
GetThatCookie:refactor
Open

http,net,stream: reconcile read, write, and parser paths#64980
GetThatCookie wants to merge 3 commits into
nodejs:mainfrom
GetThatCookie:refactor

Conversation

@GetThatCookie

@GetThatCookie GetThatCookie commented Aug 3, 2026

Copy link
Copy Markdown

This PR reconciles the overlapping goals of #64887, #64346, #64393, and
#64455 into one canonical HTTP, net, and stream architecture.

Those PRs identified real bottlenecks and directly informed this work. The
goal here is to combine their valid objectives without landing competing
response builders, caches, fast paths, or duplicate lifecycle state.

No public API or documented wire behavior is intentionally changed.

Changes

790260a488e — canonical write architecture

  • batches compatible HTTP framing, payloads, trailers, and terminating chunks
    through one shared vector path;
  • fixes corked-output ordering while preserving callbacks, backpressure,
    destruction, TLS, trailers, and pipelining;
  • creates WriteWrap objects only for asynchronous writes;
  • coalesces adjacent native string storage safely;
  • preserves partial writes, errors, IPC, async hooks, and custom transports;
  • caches maxHeaderPairs once per independent parser header section.

bb58fcd9904 — final path reconciliation

  • reuses the existing send path for eligible final strings, Buffers, and
    Uint8Array values;
  • preserves mutable Buffer and custom write/_writev semantics;
  • applies one capability decision to immediate and queued output;
  • delegates compatible HTTP/2 final writes to Http2Stream.end(chunk);
  • adds focused HTTP/1, HTTP/2, net, and stream regression coverage.

39318a1d642 — bounded managed-read reuse

  • reuses one unexposed 64 KiB read backing store per Environment;
  • retains the existing managed-buffer map as the ownership source;
  • preserves exact chunk sizes and independent transferability;
  • avoids shared slabs, backing-store amplification, and cross-chunk detach
    coupling.

Related work

This PR is intended to supersede the overlapping work while acknowledging the
analysis and implementation effort behind it:

PR Reconciliation
#64887 Its corked-output and parser-section work is contained here and extended by the shared vector architecture.
#64346 Its outgoing-overhead objective is covered without retaining isolated micro-hoists or additional per-message cache state.
#64393 Its single-shot objective is covered without introducing a second native response builder or complete-response cache.
#64455 Its lazy-WriteWrap work is integrated, while its read-allocation objective is addressed through bounded internal reuse instead of a JavaScript-visible shared slab.

The tested #64393 tip returned stale output when a reused Buffer changed from
A to B: main and this branch produced A, B, while that branch produced
A, A. Its cached headline path is therefore not a semantics-preserving
comparison target.

The tested shared slab from #64455 exposed a 64 KiB backing store for small
chunks and coupled their transfer/detach behavior. This branch keeps every
emitted chunk exactly sized and independently transferable.

Performance

Local macOS arm64 Release builds were compared through paired, interleaved
runs against the same f43086d1e4d base.

Workload Result
All 33 HTTP output configurations all positive
Geometric HTTP gain +68.11%
Geometric gain excluding the previously broken explicit-cork baseline +29.65%
HTTP explicit cork, 16 chunks 17.6x to 30.2x mean throughput; up to 31.0x median
HTTPS explicit cork, 16 chunks 1,814 → 48,060 req/s (26.5x)
HTTP auto-cork, 16 chunks +47.39% to +57.70%
HTTP fixed body, 128 chunks +154.61% to +168.57%
HTTPS auto-cork, 16 chunks +41.83%
HTTPS fixed body, 128 chunks +88.57%
Parser, 4–32 filler headers +17.71% to +54.14%
End-to-end outgoing headers +1.96% to +10.48%
Serial 1 KiB TCP roundtrips 52,601 → 55,977/s (+6.42%)
Saturated read throughput neutral (+0.08%)

The exceptional explicit-cork result comes from correcting a path that
previously emitted HTTP framing writes outside the socket-level batch. It is
the removal of a pathological unbatched path, not a claim that every HTTP
workload becomes 30x faster.

The read cache retains exactly 65,536 additional arrayBuffers bytes per
Environment. That value remained constant from 5,000 through 200,000
roundtrips, with no growth in RSS or external memory.

Validation

The output/write reconciliation passed:

  • HTTP, HTTPS, and HTTP/2: 831/831
  • net, stream, and TLS: 638/638
  • child process, cluster, async hooks, and Web Streams: 243/243
  • sequential HTTP, net, pipe, and GC tests: 25/25

The final read-buffer binary additionally passed:

  • net and TLS: 382/382
  • branch-specific changed-path tests: 20/20
  • Environment C++ tests: 28/28
  • independent TLS transfer/detach verification
  • JavaScript and C++ lint
  • git diff --check

Documentation is unchanged because this is an internal, behavior-preserving
reconciliation.

AI assistance

I developed this PR iteratively with OpenAI Codex as a pair-programming and
analysis tool. It helped trace the affected code paths and related PRs,
explore implementation alternatives, expand test coverage, run and analyze
benchmarks, and edit parts of this description.

I evaluated the alternatives, removed candidates that did not preserve
semantics or demonstrate measurable value, and reviewed and understand the
final diff and reported results.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/http2
  • @nodejs/net
  • @nodejs/performance
  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 3, 2026
@avivkeller

Copy link
Copy Markdown
Member

Please sign your comments, and add disclosers for any and all AI usage

@ronag
ronag requested review from jasnell and mcollina August 3, 2026 16:45
@ronag

ronag commented Aug 3, 2026

Copy link
Copy Markdown
Member

Looks AI generated... interesting non-the-less... not sure what to think

Signed-off-by: GetThatCookie <NimmenKeks@gmx.de>
Signed-off-by: GetThatCookie <NimmenKeks@gmx.de>
Signed-off-by: GetThatCookie <NimmenKeks@gmx.de>
@GetThatCookie

Copy link
Copy Markdown
Author

Just for some context... I use node in our own project and stumbled across the
cork/uncork behavior more or less by accident. While digging into it, I found
several open PRs touching the same area.

My intention was simply to see if those ideas could be brought together
without adding more separate fast paths, duplicated state, and unclear
ownership. I am not trying to dismiss anyone's work or step on anyone's toes.

If the PR is too large or just not the direction you want to take, I am
also completely fine with it being closed.

@ronag

ronag commented Aug 3, 2026

Copy link
Copy Markdown
Member

As I said, I think this is interesting

@ronag ronag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we move the http2 stuff to separate PR? I'm not comfortable reviewing that

@ronag ronag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the stream changes. Can those be commented/explained?

@ronag ronag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would help to have the 3 commits as separate PRs? I also think they are not entirely dependent?

@GetThatCookie

Copy link
Copy Markdown
Author

@ronag I split this into four PRs now. A 1:1 split of the three commits wouldn't work because the second one completes the first. The parser and HTTP/2 parts are independent - your guess was right here.

I also added a few comments around the stream changes.

In short, the vector still goes through Writable so ordering, backpressure, and callbacks stay intact. StreamBase avoids creating a WriteWrap for synchronous writes and safely handles completion before JavaScript has attached the callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants